check return type of this type predicates#57341
Conversation
|
@typescript-bot test top200 @typescript-bot perf test this faster |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 89a7208. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 89a7208. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 89a7208. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 89a7208. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the faster perf test suite on this PR at 89a7208. You can monitor the build here. Update: The results are in! |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@jakebailey Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey @jakebailey, the results of running the DT tests are ready. |
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
src/compiler/checker.ts
Outdated
| return Ternary.False; | ||
| } | ||
| else if (isThisTypePredicate(targetTypePredicate)) { | ||
| result &= compareTypes(sourceReturnType, targetReturnType, reportErrors); |
There was a problem hiding this comment.
That doesn't look right. We don't consider a function with a boolean return type assignable to a function with an identifier type predicate, as the code above implies. So I think instead of adding this change, what we want to do is change the condition above to also account for this type predicates: they should behave the same as identifier type predicates.
There was a problem hiding this comment.
Updated. However, it could potentially break some seemingly innocent code.
interface LineCollection {
isLeaft(): this is LineLeaf;
}
class LineNode implements LineCollection {
isLeaf() {
return false;
}
}| !!! error TS2416: Type '() => void' is not assignable to type '() => boolean'. | ||
| !!! error TS2416: Type 'void' is not assignable to type 'boolean'. | ||
|
|
||
| method3() { // should ok |
| } | ||
|
|
||
| isLeaf() { | ||
| isLeaf(): this is LineLeaf { |
|
@typescript-bot test it |
|
Hey @gabritto, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: vinyl-file Package: vinyl Package: winjs |
|
@gabritto Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@gabritto Here are the results of running the user tests comparing Everything looks good! |
|
@gabritto Here are the results of running the top 400 repos comparing Something interesting changed - please have a look. Details
|

Fixes #57103